-
Notifications
You must be signed in to change notification settings - Fork 82
New issue
Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.
By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.
Already on GitHub? Sign in to your account
Implement Media Sink Wants in Voice #898
base: main
Are you sure you want to change the base?
Conversation
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
please run ./gradlew apiDump
i'm curious, are there any resources to read up on this feature? it isn't documented in the discord developer portal yet |
It isn't documented, just like voice receive (which is supported by kord voice), this was discovered through reverse engineering the official Discord Client. |
@@ -21,6 +21,7 @@ public class DefaultVoiceGatewayBuilder( | |||
public var client: HttpClient? = null | |||
public var reconnectRetry: Retry? = null | |||
public var eventFlow: MutableSharedFlow<VoiceEvent> = MutableSharedFlow(extraBufferCapacity = Int.MAX_VALUE) | |||
public var isDeaf: Boolean = false |
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
VoiceConnectionBuilder
already has the selfDeaf
option, can't this be used instead?
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
I suppose. I think the VoiceConnectionBuilder
could get cleaned up quite a bit though.
Like I state in the PR description, you can set selfDeaf
and a streams
implementation.
Since selfDeaf
is just a UI update right now it's not that much of an issue but if it gets updated to actually prevent voice receive it might be a good idea to set it depending on the streams implementation so that there isn't any confusion.
And maybe add some more functionality to it as well:
interface Streams {
suspend fun setDeafen(value: Boolean)
suspend fun setMute(ssrc: UInt, value: Boolean)
}
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
I think this is a fair solution. Makes the deafening much behavior much more consistent, especially with this addition. Streams#setDeafen might not be needed, as it isn't provided for the UI deafen either. I could see the use for a VoiceConnection#(set)Deafen which would UI deafen and media sink deafen (if streams are open).
Functionality-wise LGTM. |
Adds a new voice gateway command (op code
15
) that controls whether the voice server sends voice packets to the bot.Still a bit unsure on the API since Kord voice allows setting the deafen value when joining and the streams implementation to use.